executor: Don't run background tasks if future is ready#2298
Conversation
|
Thanks for bringing back this patch! I'm concerned the fact that the executor may run way less often may cause other issues. Then again, the |
Yeah, I think this is happening in CI. Of course, one of the issues with our executor is that many things are actually blocking, which is bad. Making things properly non-blocking should make polling the executor here a non-issue. On the other hand, I agree that if something breaks due to this PR, that is probably a bug and should be resolved. |
There was a problem hiding this comment.
Benchmark Results
Details
| Benchmark | Current: a27a4e9 | Previous: 28c7089 | Performance Ratio |
|---|---|---|---|
| startup_benchmark Build Time | 124.99 s |
115.28 s |
1.08 ❗ |
| startup_benchmark File Size | 0.78 MB |
0.77 MB |
1.02 ❗ |
| Startup Time - 1 core | 1.02 s (±0.04 s) |
1.00 s (±0.03 s) |
1.02 |
| Startup Time - 2 cores | 1.01 s (±0.02 s) |
1.04 s (±0.05 s) |
0.98 |
| Startup Time - 4 cores | 0.87 s (±0.12 s) |
1.00 s (±0.06 s) |
0.88 |
| multithreaded_benchmark Build Time | 113.04 s |
118.33 s |
0.96 ❗ |
| multithreaded_benchmark File Size | 0.89 MB |
0.87 MB |
1.02 ❗ |
| Multithreaded Pi Efficiency - 2 Threads | 88.78 % (±15.36 %) |
92.34 % (±11.70 %) |
0.96 |
| Multithreaded Pi Efficiency - 4 Threads | 44.72 % (±6.66 %) |
45.83 % (±5.87 %) |
0.98 |
| Multithreaded Pi Efficiency - 8 Threads | 24.70 % (±3.87 %) |
25.71 % (±3.50 %) |
0.96 |
| micro_benchmarks Build Time | 92.31 s |
96.08 s |
0.96 ❗ |
| micro_benchmarks File Size | 0.89 MB |
0.88 MB |
1.02 ❗ |
| Scheduling time - 1 thread | 72.76 ticks (±3.02 ticks) |
73.85 ticks (±3.74 ticks) |
0.99 |
| Scheduling time - 2 threads | 39.66 ticks (±3.88 ticks) |
41.46 ticks (±4.37 ticks) |
0.96 |
| Micro - Time for syscall (getpid) | 3.07 ticks (±0.24 ticks) |
3.08 ticks (±0.19 ticks) |
1.00 |
| Memcpy speed - (built_in) block size 4096 | 74568.17 MByte/s (±51578.65 MByte/s) |
73208.48 MByte/s (±50616.82 MByte/s) |
1.02 |
| Memcpy speed - (built_in) block size 1048576 | 29306.44 MByte/s (±24108.75 MByte/s) |
29392.18 MByte/s (±24125.47 MByte/s) |
1.00 |
| Memcpy speed - (built_in) block size 16777216 | 22320.45 MByte/s (±18274.18 MByte/s) |
23751.74 MByte/s (±19605.56 MByte/s) |
0.94 |
| Memset speed - (built_in) block size 4096 | 75078.86 MByte/s (±51941.51 MByte/s) |
73313.38 MByte/s (±50689.78 MByte/s) |
1.02 |
| Memset speed - (built_in) block size 1048576 | 30067.71 MByte/s (±24538.62 MByte/s) |
30169.43 MByte/s (±24587.38 MByte/s) |
1.00 |
| Memset speed - (built_in) block size 16777216 | 22769.89 MByte/s (±18509.02 MByte/s) |
24402.29 MByte/s (±20008.02 MByte/s) |
0.93 |
| Memcpy speed - (rust) block size 4096 | 66044.57 MByte/s (±46181.55 MByte/s) |
66427.37 MByte/s (±46490.49 MByte/s) |
0.99 |
| Memcpy speed - (rust) block size 1048576 | 29390.56 MByte/s (±24142.20 MByte/s) |
29425.11 MByte/s (±24175.79 MByte/s) |
1.00 |
| Memcpy speed - (rust) block size 16777216 | 23045.55 MByte/s (±19049.98 MByte/s) |
24403.77 MByte/s (±20179.96 MByte/s) |
0.94 |
| Memset speed - (rust) block size 4096 | 66468.10 MByte/s (±46474.60 MByte/s) |
66262.32 MByte/s (±46403.75 MByte/s) |
1.00 |
| Memset speed - (rust) block size 1048576 | 30145.18 MByte/s (±24582.74 MByte/s) |
30172.11 MByte/s (±24599.55 MByte/s) |
1.00 |
| Memset speed - (rust) block size 16777216 | 23766.69 MByte/s (±19545.72 MByte/s) |
25069.95 MByte/s (±20590.63 MByte/s) |
0.95 |
| alloc_benchmarks Build Time | 92.02 s |
91.43 s |
1.01 ❗ |
| alloc_benchmarks File Size | 0.86 MB |
0.85 MB |
1.02 ❗ |
| Allocations - Allocation success | 100.00 % |
100.00 % |
1 |
| Allocations - Deallocation success | 100.00 % |
100.00 % |
1 |
| Allocations - Pre-fail Allocations | 100.00 % |
100.00 % |
1 |
| Allocations - Average Allocation time | 4837.63 Ticks (±548.83 Ticks) |
4633.96 Ticks (±62.19 Ticks) |
1.04 |
| Allocations - Average Allocation time (no fail) | 4837.63 Ticks (±548.83 Ticks) |
4633.96 Ticks (±62.19 Ticks) |
1.04 |
| Allocations - Average Deallocation time | 760.95 Ticks (±158.83 Ticks) |
683.28 Ticks (±99.25 Ticks) |
1.11 |
| mutex_benchmark Build Time | 94.80 s |
90.68 s |
1.05 ❗ |
| mutex_benchmark File Size | 0.90 MB |
0.88 MB |
1.02 ❗ |
| Mutex Stress Test Average Time per Iteration - 1 Threads | 13.82 ns (±0.77 ns) |
13.82 ns (±0.84 ns) |
1.00 |
| Mutex Stress Test Average Time per Iteration - 2 Threads | 16.58 ns (±0.80 ns) |
14.48 ns (±0.75 ns) |
1.15 ❗ |
This comment was automatically generated by workflow using github-action-benchmark.
|
As far as I can tell, the CI issue is the same as #2292, I can reproduce it locally and am debugging it |
Often times, the future is ready immediately and yet we will still run the background tasks every time, adding significant overhead that is noticable when benchmarking a HTTP server. The blocking httpd example gains anywhere between 20-50% more throughput for me with these changes, while the netbench TCP bandwidth benchmark only slightly regresses by about 5%. The httpd example ported to axum sees an increase in throughput by 500-600%. Co-authored-by: Louis Vialar <louis.vialar@gmail.com>
e67a10f to
a27a4e9
Compare
Often times, the future is ready immediately and yet we will still run the background tasks every time, adding significant overhead that is noticable when benchmarking a HTTP server.
The blocking httpd example gains anywhere between 20-50% more throughput for me with these changes, while the netbench TCP bandwidth benchmark only slightly regresses by about 5%. The httpd example ported to axum sees an increase in throughput by 500-600%:
before:
after:
This was suggested by @zyuiop in #2086 (comment).